-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🚀 feat(PlanCard): Make ListItem clickable #670
Conversation
display: flex; | ||
min-width: 0; | ||
align-items: center; | ||
|
||
${props => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clicks can also be a link, Should we add that option? Example:
If the component receives a href
prop, we can assume element should be a <a>
tag..
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, beware that button
should have the "type" attribute. (Maybe we can assume always type="button"
).
A helpful article: https://www.builder.io/blog/buttons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clicks can also be a link, Should we add that option? Example: If the component receives a
href
prop, we can assume element should be a<a>
tag..WDYT?
I have discussed with the team, and for now, links are out of scope, at least within the context of Gympass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, beware that
button
should have the "type" attribute. (Maybe we can assume alwaystype="button"
).A helpful article: https://www.builder.io/blog/buttons
Nice article! I completely agree. I added the button type in this commit: 5845523.
@MicaelRodrigues could you review it again please?
Description 📄
onClick
toPlanCard.ListItem
component and transform the wrapper in a button to make it accessibletext
, it should accept components such asSkeleton
Platforms 📲
Type of change 🔍
How Has This Been Tested? 🧪
[Enter the tips to test this PR]
Checklist: 🔍
Screenshots 📸
No visual changes.